Fix macOS compatibility in tools/log.sh#741
Fix macOS compatibility in tools/log.sh#741Kunal241207 wants to merge 2 commits intoprecice:developfrom
Conversation
|
@Kunal241207 nice fix. two quick questions: (1) Was using date -u in the fallback intentional? on linux we get local time, so i was wondering if we’d want the same on macOS , (may be removing that -u) (2) One “Started on:” has two spaces after the colon and the other has one—was that on purpose? Just curious. thanks |
|
@PranjalManhgaye Good catch, thanks!
I've pushed the update. |
MakisH
added
GSoC
|
Ah, this has been in my list since longer, thanks! Related issue: #601 Do you think this approach solves both? |
|
This change makes the timestamp generation in tools/log.sh portable, so it avoids the |
MakisH
left a comment
MakisH
left a comment
There was a problem hiding this comment.
I just had another look. Overall, this PR not there yet, but it can be improved.
There are no other places where this is used.
|
|
||
| STARTDATE="$(date --rfc-email)" | ||
| rfc_email_date() { | ||
| date --rfc-email 2>/dev/null || date '+%a, %d %b %Y %H:%M:%S %z' |
There was a problem hiding this comment.
Why the 2>/dev/null? If there is an error, I would rather see it than get the script to fail silently.
| export LOGFILE | ||
|
|
||
| STARTDATE="$(date --rfc-email)" | ||
| rfc_email_date() { |
There was a problem hiding this comment.
The rfc_email_date directly refers to the implementation options (--rfc-email), which is the non-portable part. A more general name would be better.
|
|
||
| STARTDATE="$(date --rfc-email)" | ||
| rfc_email_date() { | ||
| date --rfc-email 2>/dev/null || date '+%a, %d %b %Y %H:%M:%S %z' |
There was a problem hiding this comment.
If --rfc-email is not portable, why provide an explicit format as fallback, instead of making the explicit format the only option? That should work on every system.
| ENDTIME="$(date +%s)" | ||
| echo "Finished on: $ENDDATE" | tee --append "$LOGFILE" 2>&1 | ||
| echo "Duration: $((ENDTIME-STARTTIME)) seconds (wall-clock time, including time waiting for participants)" | tee --append "$LOGFILE" 2>&1 | ||
| echo "Finished on: $ENDDATE" | tee -a "$LOGFILE" 2>&1 |
There was a problem hiding this comment.
What's wrong with --apend? I used the verbose option for readability.
I don't have a macOS system to test on, but this page, for example, suggests that both are valid.
| echo "Finished on: $ENDDATE" | tee --append "$LOGFILE" 2>&1 | ||
| echo "Duration: $((ENDTIME-STARTTIME)) seconds (wall-clock time, including time waiting for participants)" | tee --append "$LOGFILE" 2>&1 | ||
| echo "Finished on: $ENDDATE" | tee -a "$LOGFILE" 2>&1 | ||
| echo "Duration: $((ENDTIME-STARTTIME)) seconds (wall-clock time, including time waiting for participants)" | tee -a "$LOGFILE" 2>&1 |
There was a problem hiding this comment.
Could you post some example output from this script, to see the format and the computed time?
3 participants
Summary
tools/log.sh used GNU-specific flags that are not available on macOS.
This change makes the script portable by:
date --rfc-emailwith a helper that falls back to a compatibledateformat when the flag is unavailable.tee --appendwith the portabletee -a.The behavior of the script remains the same, but it now works correctly on macOS systems.
Checklist:
changelog-entries/<PRnumber>.md.